Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a memoized Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cozy-search/src/components/CreateAssistantSteps/useAssistantDialog.js (1)
64-70:⚠️ Potential issue | 🟠 MajorReset credentials when provider changes to avoid stale-key bypass
When provider changes,
apiKey/encryptedApiKeyare preserved. In edit flow, Line 140 inpackages/cozy-search/src/components/Views/EditAssistantDialog.jsxuses!!formData.encryptedApiKeyto allow skipping API key entry, so stale credentials from a previous provider can incorrectly bypass validation.💡 Proposed fix
const handleProviderSelection = provider => { + const hasProviderChanged = selectedProvider?.id !== provider.id setFormData(prev => ({ ...prev, baseUrl: provider.baseUrl, isCustomModel: provider.id === 'custom', - model: provider.id === 'openrag' ? provider.models[0] : prev.model + model: provider.id === 'openrag' ? provider.models[0] : prev.model, + ...(hasProviderChanged ? { apiKey: '', encryptedApiKey: '' } : {}) })) setSelectedProvider({ ...provider, name: provider.id === 'custom' ? undefined : provider.name }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-search/src/components/CreateAssistantSteps/useAssistantDialog.js` around lines 64 - 70, handleProviderSelection currently preserves apiKey/encryptedApiKey, allowing stale credentials to bypass validation in edit flow; update setFormData inside handleProviderSelection to also clear credential fields (set apiKey to an empty string and encryptedApiKey to a falsy value) whenever the provider changes so EditAssistantDialog.jsx's !!formData.encryptedApiKey cannot be satisfied by a previous provider's key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@packages/cozy-search/src/components/CreateAssistantSteps/useAssistantDialog.js`:
- Around line 64-70: handleProviderSelection currently preserves
apiKey/encryptedApiKey, allowing stale credentials to bypass validation in edit
flow; update setFormData inside handleProviderSelection to also clear credential
fields (set apiKey to an empty string and encryptedApiKey to a falsy value)
whenever the provider changes so EditAssistantDialog.jsx's
!!formData.encryptedApiKey cannot be satisfied by a previous provider's key.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/cozy-search/src/components/CreateAssistantSteps/useAssistantDialog.jspackages/cozy-search/src/components/Views/CreateAssistantDialog.jsxpackages/cozy-search/src/components/Views/EditAssistantDialog.jsx
| step === STEPS.API_KEY || | ||
| (step === STEPS.MODEL_SELECTION && selectedProvider?.id === 'openrag'), | ||
| [step, selectedProvider?.id] | ||
| ) |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx (1)
37-46:⚠️ Potential issue | 🟠 MajorBug: Provider name field updates incorrect formData field.
The TextField for provider name in custom provider mode uses
onChange('model'), butformData.modelis reserved for model selection in non-custom mode. This is a semantic mismatch. The provider name field should either be read-only or update a dedicated field. Check whether the provider name should be user-editable for custom providers, as the current implementation updates the wrong form field and could cause data corruption during submission.Current problematic code
<TextField fullWidth placeholder={t( 'assistant_create.steps.configuration.provider.placeholder' )} value={providerName} onChange={onChange('model')} variant="outlined" type="text" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx` around lines 37 - 46, The TextField for provider name is incorrectly wired to onChange('model') which mutates formData.model; change it to update a dedicated field (e.g. onChange('providerName') or onChange('name')) or make the field readOnly depending on desired UX. Locate the TextField in ApiKeyStep.jsx (props/locals: providerName, onChange) and: if providerName should be editable, add/use a separate form key (providerName/name) in formData and replace onChange('model') with onChange('providerName') (and update any form handling to include that key); if it should be fixed, set the TextField to readOnly/disabled and remove the onChange handler to prevent accidentally mutating formData.model. Ensure form submission and validation use the new dedicated field instead of formData.model.
🧹 Nitpick comments (1)
packages/cozy-search/src/locales/en.json (1)
130-131: Tighten the placeholder copy.
Enter the model name of the providerreads a bit awkwardly in English and can blur provider vs. model.Enter the model nameorEnter the provider's model namewould be clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-search/src/locales/en.json` around lines 130 - 131, The placeholder text for the "placeholder" field under the "label": "Model" entry is awkward and can confuse provider vs model; update the placeholder string (the "placeholder" key in the same JSON entry) to a clearer variant such as "Enter the model name" or "Enter the provider's model name" to tighten copy and remove ambiguity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx`:
- Around line 37-46: The TextField for provider name is incorrectly wired to
onChange('model') which mutates formData.model; change it to update a dedicated
field (e.g. onChange('providerName') or onChange('name')) or make the field
readOnly depending on desired UX. Locate the TextField in ApiKeyStep.jsx
(props/locals: providerName, onChange) and: if providerName should be editable,
add/use a separate form key (providerName/name) in formData and replace
onChange('model') with onChange('providerName') (and update any form handling to
include that key); if it should be fixed, set the TextField to readOnly/disabled
and remove the onChange handler to prevent accidentally mutating formData.model.
Ensure form submission and validation use the new dedicated field instead of
formData.model.
---
Nitpick comments:
In `@packages/cozy-search/src/locales/en.json`:
- Around line 130-131: The placeholder text for the "placeholder" field under
the "label": "Model" entry is awkward and can confuse provider vs model; update
the placeholder string (the "placeholder" key in the same JSON entry) to a
clearer variant such as "Enter the model name" or "Enter the provider's model
name" to tighten copy and remove ambiguity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 66a66702-07f7-412a-ae9a-747974e927da
📒 Files selected for processing (12)
packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsxpackages/cozy-search/src/components/CreateAssistantSteps/Provider.jsxpackages/cozy-search/src/components/CreateAssistantSteps/helpers.jspackages/cozy-search/src/components/CreateAssistantSteps/providers.jsonpackages/cozy-search/src/components/CreateAssistantSteps/styles.stylpackages/cozy-search/src/components/CreateAssistantSteps/useAssistantDialog.jspackages/cozy-search/src/components/Views/CreateAssistantDialog.jsxpackages/cozy-search/src/components/Views/EditAssistantDialog.jsxpackages/cozy-search/src/locales/en.jsonpackages/cozy-search/src/locales/fr.jsonpackages/cozy-search/src/locales/ru.jsonpackages/cozy-search/src/locales/vi.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cozy-search/src/components/Views/EditAssistantDialog.jsx
- packages/cozy-search/src/components/CreateAssistantSteps/useAssistantDialog.js
0d08e1f to
511ce47
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx`:
- Line 18: The inputs for provider name, model, and baseUrl are rendering from
selectedProvider but updates only write to formData, causing the UI and
submitted payload to diverge; update the ApiKeyStep component so these inputs
are driven from formData (not directly from selectedProvider) or ensure
selectedProvider is updated when the user edits the fields. Concretely, change
the value props for the name/model/baseUrl inputs (currently using
selectedProvider or destructured id,name,model,baseUrl) to read from formData
(e.g. formData.providerName/formData.model/formData.baseUrl) and keep onChange
handlers updating formData, or alternatively wire the existing model-selection
update path used by AssistantDialogContent (the model-selection handlers around
AssistantDialogContent.jsx:35-42) to also set selectedProvider when the form
inputs change; make the same change for all three input groups referenced (lines
around 37-46, 55-64, 73-81) so displayed values and the submitted payload remain
in sync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf5121d7-fe43-4d2c-9af7-797b5d2fe2b0
📒 Files selected for processing (12)
packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsxpackages/cozy-search/src/components/CreateAssistantSteps/Provider.jsxpackages/cozy-search/src/components/CreateAssistantSteps/helpers.jspackages/cozy-search/src/components/CreateAssistantSteps/providers.jsonpackages/cozy-search/src/components/CreateAssistantSteps/styles.stylpackages/cozy-search/src/components/CreateAssistantSteps/useAssistantDialog.jspackages/cozy-search/src/components/Views/CreateAssistantDialog.jsxpackages/cozy-search/src/components/Views/EditAssistantDialog.jsxpackages/cozy-search/src/locales/en.jsonpackages/cozy-search/src/locales/fr.jsonpackages/cozy-search/src/locales/ru.jsonpackages/cozy-search/src/locales/vi.json
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/cozy-search/src/locales/fr.json
- packages/cozy-search/src/components/CreateAssistantSteps/Provider.jsx
- packages/cozy-search/src/locales/vi.json
- packages/cozy-search/src/components/CreateAssistantSteps/styles.styl
- packages/cozy-search/src/locales/en.json
- packages/cozy-search/src/locales/ru.json
- packages/cozy-search/src/components/Views/EditAssistantDialog.jsx
- packages/cozy-search/src/components/CreateAssistantSteps/providers.json
- packages/cozy-search/src/components/CreateAssistantSteps/helpers.js
- packages/cozy-search/src/components/Views/CreateAssistantDialog.jsx
35dd4a4 to
3d1376d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx (1)
37-46:⚠️ Potential issue | 🟠 MajorControlled input mismatch:
valueandonChangetarget different state.
value={providerName}reads fromselectedProvider.name, butonChange={onChange('model')}writes toformData.model. When the user types,formData.modelupdates but the TextField continues displaying the unchangedselectedProvider.name, making the input appear frozen.If these fields are display-only, make the intent explicit:
<TextField fullWidth placeholder={...} value={providerName} - onChange={onChange('model')} variant="outlined" type="text" + InputProps={{ readOnly: true }} />If editable, bind
valuetoformData:-value={providerName} +value={formData.model}The same issue applies to the
baseUrlfield (lines 55-64) andmodelfield (lines 73-82).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx` around lines 37 - 46, The TextField for provider currently reads from providerName (selectedProvider.name) but writes to formData.model via onChange('model'), causing a controlled input mismatch; update bindings so read and write target the same state: either make the field display-only by keeping value={providerName} and remove or replace onChange with a no-op/display handler, or make it editable by changing value to the corresponding formData property (e.g., value={formData.model}) and keep onChange={onChange('model')}; apply the same fix to the baseUrl and model TextField instances so each TextField's value prop and onChange refer to the same state identifiers (selectedProvider.name vs formData.baseUrl/formData.model).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx`:
- Line 13: AssistantDialogContent is still passing the removed onModelSelect
prop into the ApiKeyStep component; remove the unused prop from the ApiKeyStep
invocation in AssistantDialogContent.jsx so the call matches ApiKeyStep's
signature ({ apiKey, selectedProvider, onChange }). Locate the JSX element that
renders <ApiKeyStep ... /> in AssistantDialogContent and delete the
onModelSelect={...} attribute (or any variable passed as onModelSelect) to stop
passing the dead prop and eliminate prop warnings.
In `@packages/cozy-search/src/components/CreateAssistantSteps/providers.json`:
- Around line 3-8: The provider entry for "openrag" is missing the baseUrl
property which causes useAssistantDialog.handleProviderSelection to copy an
undefined provider.baseUrl into formData and CreateAssistantDialog to submit
formData.baseUrl as undefined; either restore a baseUrl field in the "openrag"
object inside providers.json (e.g., add "baseUrl": "<Twake endpoint>") or update
useAssistantDialog.handleProviderSelection and CreateAssistantDialog to stop
copying/sending baseUrl and instead derive the endpoint solely from providerId
("openrag"); modify the code paths referenced by
useAssistantDialog.handleProviderSelection and CreateAssistantDialog to keep
behavior consistent with the chosen approach.
In `@packages/cozy-search/src/components/Views/CreateAssistantDialog.jsx`:
- Around line 64-65: When building the submit payload in
CreateAssistantDialog.jsx (inside the onSubmit handler) guard against a null
selectedProvider before accessing selectedProvider.id; update the payload
creation that currently uses selectedProvider.id to use a safe access pattern
(e.g., optional chaining selectedProvider?.id or fallback value) and/or validate
selectedProvider and early-return or show an error if it is null, so
formData.isCustomModel and providerId are always set without throwing.
---
Outside diff comments:
In `@packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx`:
- Around line 37-46: The TextField for provider currently reads from
providerName (selectedProvider.name) but writes to formData.model via
onChange('model'), causing a controlled input mismatch; update bindings so read
and write target the same state: either make the field display-only by keeping
value={providerName} and remove or replace onChange with a no-op/display
handler, or make it editable by changing value to the corresponding formData
property (e.g., value={formData.model}) and keep onChange={onChange('model')};
apply the same fix to the baseUrl and model TextField instances so each
TextField's value prop and onChange refer to the same state identifiers
(selectedProvider.name vs formData.baseUrl/formData.model).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9ffd1103-232c-440e-bc47-5685e6646f19
📒 Files selected for processing (12)
packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsxpackages/cozy-search/src/components/CreateAssistantSteps/Provider.jsxpackages/cozy-search/src/components/CreateAssistantSteps/helpers.jspackages/cozy-search/src/components/CreateAssistantSteps/providers.jsonpackages/cozy-search/src/components/CreateAssistantSteps/styles.stylpackages/cozy-search/src/components/CreateAssistantSteps/useAssistantDialog.jspackages/cozy-search/src/components/Views/CreateAssistantDialog.jsxpackages/cozy-search/src/components/Views/EditAssistantDialog.jsxpackages/cozy-search/src/locales/en.jsonpackages/cozy-search/src/locales/fr.jsonpackages/cozy-search/src/locales/ru.jsonpackages/cozy-search/src/locales/vi.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/cozy-search/src/locales/vi.json
- packages/cozy-search/src/components/CreateAssistantSteps/helpers.js
- packages/cozy-search/src/locales/fr.json
3d1376d to
8177224
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx (1)
18-18:⚠️ Potential issue | 🟠 MajorDrive these inputs from
formData, notselectedProvider.
onChangeonly mutatesformData, but thevalueprops here come fromselectedProvider. In edit mode,EditAssistantDialoghydratesselectedProviderwith the persisted model/base URL, so these fields become effectively read-only while the submitted payload changes underneath. Bind them to the same state you submit, or keepselectedProvidersynchronized on every edit.Possible fix
-const ApiKeyStep = ({ apiKey, selectedProvider, onChange }) => { +const ApiKeyStep = ({ apiKey, formData, selectedProvider, onChange }) => { const { type: theme } = useCozyTheme() const [showPassword, setShowPassword] = useState(false) const { t } = useI18n() const { id, name: providerName, model, baseUrl } = selectedProvider || {} @@ - value={providerName} + value={formData.model} onChange={onChange('model')} @@ - value={baseUrl} + value={formData.baseUrl} onChange={onChange('baseUrl')} @@ - value={model} + value={formData.model} onChange={onChange('model')}Also pass
formDatathrough fromAssistantDialogContent.Also applies to: 37-43, 55-61, 73-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx` at line 18, The inputs in ApiKeyStep (the destructured selectedProvider: id, providerName, model, baseUrl) are bound to selectedProvider while onChange only updates formData, causing read-only behavior in edit mode; update the input value props to read from formData (e.g., formData.providerId, formData.providerName, formData.model, formData.baseUrl) or alternatively call the same onChange handler to keep selectedProvider in sync on every edit, and ensure AssistantDialogContent/ EditAssistantDialog pass formData down into ApiKeyStep so the component binds and submits the same state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/cozy-search/src/components/CreateAssistantSteps/useAssistantDialog.js`:
- Around line 43-47: When providerId changes we must clear provider-scoped
secrets so stale apiKey/encryptedApiKey aren't submitted: update
handleProviderSelection (and the form update path used in edit mode) to reset
formData.apiKey and formData.encryptedApiKey (and any other provider-scoped
fields) whenever providerId (or selectedProvider?.id) is changed; ensure the
same clearing logic runs before canSubmit is evaluated so switching to
STEPS.MODEL_SELECTION with selectedProvider.id === 'openrag' or switching
between external providers cannot reuse the previous provider's credentials.
---
Duplicate comments:
In `@packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsx`:
- Line 18: The inputs in ApiKeyStep (the destructured selectedProvider: id,
providerName, model, baseUrl) are bound to selectedProvider while onChange only
updates formData, causing read-only behavior in edit mode; update the input
value props to read from formData (e.g., formData.providerId,
formData.providerName, formData.model, formData.baseUrl) or alternatively call
the same onChange handler to keep selectedProvider in sync on every edit, and
ensure AssistantDialogContent/ EditAssistantDialog pass formData down into
ApiKeyStep so the component binds and submits the same state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8fe47bfe-79e0-48c7-bec0-74103de473a8
📒 Files selected for processing (12)
packages/cozy-search/src/components/CreateAssistantSteps/ApiKeyStep.jsxpackages/cozy-search/src/components/CreateAssistantSteps/Provider.jsxpackages/cozy-search/src/components/CreateAssistantSteps/helpers.jspackages/cozy-search/src/components/CreateAssistantSteps/providers.jsonpackages/cozy-search/src/components/CreateAssistantSteps/styles.stylpackages/cozy-search/src/components/CreateAssistantSteps/useAssistantDialog.jspackages/cozy-search/src/components/Views/CreateAssistantDialog.jsxpackages/cozy-search/src/components/Views/EditAssistantDialog.jsxpackages/cozy-search/src/locales/en.jsonpackages/cozy-search/src/locales/fr.jsonpackages/cozy-search/src/locales/ru.jsonpackages/cozy-search/src/locales/vi.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cozy-search/src/components/Views/CreateAssistantDialog.jsx
- packages/cozy-search/src/components/CreateAssistantSteps/styles.styl
8177224 to
943ef56
Compare
f0e2501 to
aaca3da
Compare
aaca3da to
43bc594
Compare
43bc594 to
a51c772
Compare
- Resolve full openrag provider from providers.json so formData.model is initialized to "openrag" instead of "" - Extract providerId into a variable in EditAssistantDialog for readability
Need:
linagora/cozy-client#1674
Summary by CodeRabbit
New Features
Bug Fixes
Style